quic: add proper error codes & messages for QUIC failures by pimterry · Pull Request #63198 · nodejs/node

pimterry · 2026-05-08T13:31:52Z

This wraps QUIC errors, providing useful error codes and messages for the defined error code cases from both OpenSSL and ngtcp2.

Before this, QUIC errors looked like:

// Application error with an explicit reason string:
Error [ERR_QUIC_APPLICATION_ERROR]: A QUIC application error occurred. 42n [client shutdown]
    at [kFinishClose] (node:internal/quic/quic:3505:22)
    at Session.onSessionClose (node:internal/quic/quic:679:31) {
  code: 'ERR_QUIC_APPLICATION_ERROR'
}

// All our internal errors:
Error [ERR_QUIC_TRANSPORT_ERROR]: A QUIC transport error occurred. 296n [undefined]
    at [kFinishClose] (node:internal/quic/quic:3502:22)
    at Session.onSessionClose (node:internal/quic/quic:679:31) {
  code: 'ERR_QUIC_TRANSPORT_ERROR'
}

Now they look like:

Error [ERR_QUIC_APPLICATION_ERROR]: QUIC application error 42: client shutdown
    at makeQuicError (node:internal/quic/quic:999:15)
    at [kFinishClose] (node:internal/quic/quic:3552:22)
    at Session.onSessionClose (node:internal/quic/quic:681:31) {
  code: 'ERR_QUIC_APPLICATION_ERROR',
  errorCode: 42n,
  type: 'application',
  reason: 'client shutdown'
}

Error [ERR_QUIC_TRANSPORT_ERROR]: QUIC transport error handshake failure (296)
    at makeQuicError (node:internal/quic/quic:988:15)
    at [kFinishClose] (node:internal/quic/quic:3534:22)
    at Session.onSessionClose (node:internal/quic/quic:681:31) {
  code: 'ERR_QUIC_TRANSPORT_ERROR',
  errorCode: 296n,
  type: 'transport',
  errorName: 'handshake failure'
}

nodejs-github-bot · 2026-05-08T13:31:57Z

Review requested:

@nodejs/quic

codecov · 2026-05-08T15:08:23Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (3b19867) to head (6adb825).
⚠️ Report is 125 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #63198      +/-   ##
==========================================
- Coverage   91.68%   90.32%   -1.36%     
==========================================
  Files         361      730     +369     
  Lines      156230   234688   +78458     
  Branches    24021    43939   +19918     
==========================================
+ Hits       143232   211990   +68758     
- Misses      12729    14411    +1682     
- Partials      269     8287    +8018

Files with missing lines	Coverage Δ
lib/internal/errors.js	`97.65% <ø> (+4.03%)`	⬆️
lib/internal/quic/quic.js	`100.00% <100.00%> (ø)`

... and 524 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Ethan-Arrowood

this is a great change.

Ethan-Arrowood

this is a great change.

Signed-off-by: Tim Perry <pimterry@gmail.com>

pimterry · 2026-05-18T16:44:23Z

Updated to use bindingdata, rebased and force pushed because we need to update the test from #63193 (just merged) as well since that asserted on the error message.

metcoder95

lgtm

Qard

Is there some reason you're undid the use of the existing core errors system? Shouldn't we be keeping that?

Qard · 2026-05-21T05:43:39Z

+function makeQuicError(code, prefix, type, errorCode, reason, errorName) {
+  const err = new QuicError(
+    quicErrorMessage(prefix, errorCode, reason, errorName),
+    { errorCode, code, type });
+  if (reason) err.reason = reason;
+  if (errorName) err.errorName = errorName;
+  return err;
+}


Should we not keep the same error construction mechanism as the others? We lose a bunch of things by not using that. Not sure we should be taking these out of the global errors set.

This was a suggestion here, changed here.

We still use standard error codes, but we use a single QuicError class instead of the standard construction of per-code error classes: https://github.com/nodejs/node/blob/main/doc/api/quic.md#class-quicerror. This error class is used by users as well to control QUIC error behaviour, and this approach gives us consistent errors for every kind of failure at the protocol level. I don't feel strongly about it myself, but it seemed like a reasonable suggestion and having a single QUIC error class is tidy.

What do we lose by not using the standard error mechanism vs this setup?

We lose the hideStackFrames logic, which means we'll see a bunch more internals things in stack traces here which we might not want to surface, like every validator gains a stack line without this. I'm not against this change, but just want to be sure that special-casing this is acceptable.

In this case, I think the previous code didn't do this either - our validators do hide internal stack traces, but AFAICT the normal error code definitions don't do this unless you use HideStackFramesError explicitly.

It is easy to do though, and I agree it'd be a nice improvement - I've just pushed a commit that adds ErrorCaptureStackTrace(err, makeQuicError) here, which will omit these internal bits from the stack in the same way. This mirrors the equivalent setup for custom errors elsewhere like

node/lib/internal/fs/utils.js

Lines 350 to 363 in 92f48f4

function handleErrorFromBinding(ctx) {

if (ctx.errno !== undefined) { // libuv error numbers

const err = new UVException(ctx);

ErrorCaptureStackTrace(err, handleErrorFromBinding);

throw err;

}

if (ctx.error !== undefined) { // Errors created in C++ land.

// TODO(joyeecheung): currently, ctx.error are encoding errors

// usually caused by memory problems. We need to figure out proper error

// code(s) for this.

ErrorCaptureStackTrace(ctx.error, handleErrorFromBinding);

throw ctx.error;

}

}

for FS bindings.

Is that what you were looking for?

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 8, 2026

pimterry force-pushed the fix-quic-error-codes branch from 31b4709 to aefee4f Compare May 8, 2026 13:41

jasnell reviewed May 8, 2026

View reviewed changes

Comment thread lib/internal/quic/quic.js Outdated

jasnell reviewed May 8, 2026

View reviewed changes

Comment thread src/quic/data.cc

trivikr added the quic Issues and PRs related to the QUIC implementation / HTTP/3. label May 11, 2026

pimterry added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label May 11, 2026

pimterry requested a review from jasnell May 11, 2026 13:45

bjohansebas reviewed May 11, 2026

View reviewed changes

Comment thread lib/internal/errors.js

jasnell reviewed May 14, 2026

View reviewed changes

Comment thread src/quic/data.cc Outdated

jasnell reviewed May 14, 2026

View reviewed changes

Comment thread src/quic/session.cc Outdated

Ethan-Arrowood reviewed May 15, 2026

View reviewed changes

Ethan-Arrowood approved these changes May 15, 2026

View reviewed changes

pimterry added 5 commits May 18, 2026 15:42

quic: add proper error codes & messages for QUIC failures

298d7d1

Signed-off-by: Tim Perry <pimterry@gmail.com>

From review: internalize QUIC error strings

73bc7ad

Signed-off-by: Tim Perry <pimterry@gmail.com>

From review: use QuicError class for thrown QUIC transport/app errors

89f0295

Signed-off-by: Tim Perry <pimterry@gmail.com>

From review: move QUIC error names into bindingdata

aea500a

Update just-merged ALPN test too

60b6e3b

pimterry force-pushed the fix-quic-error-codes branch from 40eebfd to 60b6e3b Compare May 18, 2026 16:16

metcoder95 approved these changes May 19, 2026

View reviewed changes

Qard reviewed May 21, 2026

View reviewed changes

From review: hide internal frames from QUIC errors

6adb825

	function handleErrorFromBinding(ctx) {
	if (ctx.errno !== undefined) { // libuv error numbers
	const err = new UVException(ctx);
	ErrorCaptureStackTrace(err, handleErrorFromBinding);
	throw err;
	}
	if (ctx.error !== undefined) { // Errors created in C++ land.
	// TODO(joyeecheung): currently, ctx.error are encoding errors
	// usually caused by memory problems. We need to figure out proper error
	// code(s) for this.
	ErrorCaptureStackTrace(ctx.error, handleErrorFromBinding);
	throw ctx.error;
	}
	}

Uh oh!

Conversation

pimterry commented May 8, 2026

Uh oh!

nodejs-github-bot commented May 8, 2026

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ethan-Arrowood left a comment

Choose a reason for hiding this comment

Uh oh!

Ethan-Arrowood left a comment

Choose a reason for hiding this comment

Uh oh!

pimterry commented May 18, 2026

Uh oh!

metcoder95 left a comment

Choose a reason for hiding this comment

Uh oh!

Qard left a comment

Choose a reason for hiding this comment

Uh oh!

Qard May 21, 2026

Choose a reason for hiding this comment

Uh oh!

pimterry May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Qard May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pimterry May 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codecov Bot commented May 8, 2026 •

edited

Loading

Qard May 22, 2026 •

edited

Loading